Skip to content

Add HiDPI support to classic UI XCB windows - #1620

Open
1024th wants to merge 1 commit into
fcitx:masterfrom
1024th:fix-xcb-hidpi
Open

Add HiDPI support to classic UI XCB windows#1620
1024th wants to merge 1 commit into
fcitx:masterfrom
1024th:fix-xcb-hidpi

Conversation

@1024th

@1024th 1024th commented Jul 31, 2026

Copy link
Copy Markdown

This improves HiDPI support in the classic UI XCB path.

Theme margins, overlays, action images, input window layout metrics, menu spacing, menu icons, separators, click regions, blur regions, and masks are now scaled with the current DPI.

This keeps the input window and XCB menu visually consistent on high-DPI displays instead of mixing scaled text with unscaled theme metrics.

Comparison on my KDE Wayland 175% scale setting, with Xft.dpi: 168 for Xwayland:
Wayland:
图片

Xwayland, before this fix:
图片

Xwayland, after this fix:
图片

Summary by CodeRabbit

  • Bug Fixes
    • Improved HiDPI support across classic menus and windows.
    • Corrected window sizing, rendering, shadows, blur effects, and popup placement on scaled displays.
    • Improved pointer hit testing and submenu interactions when display scaling is enabled.
    • Updated text and theme element scaling for clearer, more consistent appearance across different DPI settings.

@coderabbitai

coderabbitai Bot commented Jul 31, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 7c0a45e4-10f8-474b-83b3-2e002004a43f

📥 Commits

Reviewing files that changed from the base of the PR and between cf54a74 and 9ae2742.

📒 Files selected for processing (6)
  • src/ui/classic/xcbinputwindow.cpp
  • src/ui/classic/xcbinputwindow.h
  • src/ui/classic/xcbmenu.cpp
  • src/ui/classic/xcbmenu.h
  • src/ui/classic/xcbwindow.cpp
  • src/ui/classic/xcbwindow.h
🚧 Files skipped from review as they are similar to previous changes (4)
  • src/ui/classic/xcbinputwindow.h
  • src/ui/classic/xcbmenu.h
  • src/ui/classic/xcbmenu.cpp
  • src/ui/classic/xcbwindow.h

📝 Walkthrough

Walkthrough

HiDPI support now converts logical geometry and input coordinates to physical pixels across classic XCB windows, input windows, and menus. Cairo uses device scaling, while blur regions, theme metrics, resizing, hit testing, and popup placement use the appropriate coordinate units.

Changes

Classic UI HiDPI scaling

Layer / File(s) Summary
XCBWindow physical geometry foundation
src/ui/classic/xcbwindow.h, src/ui/classic/xcbwindow.cpp
Adds DPI-based conversion helpers and uses physical dimensions for XCB windows, Cairo surfaces, resizing, and prerendering.
XCBInputWindow scaled layout and input
src/ui/classic/xcbinputwindow.h, src/ui/classic/xcbinputwindow.cpp
Scales shadow and blur geometry, converts pointer coordinates to logical units, and derives the window scale from DPI.
XCBMenu scaled layout and placement
src/ui/classic/xcbmenu.h, src/ui/classic/xcbmenu.cpp
Converts menu coordinates and rectangles, preserves logical Pango resolution, and uses physical dimensions for layout and popup placement.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

  • fcitx/fcitx5#1615: Directly related HiDPI and rendering changes in the same XCB components.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: adding HiDPI support to classic UI XCB windows.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/ui/classic/inputwindow.cpp`:
- Around line 677-680: The Yoga layout calculation for button width combines the
individual action dimensions without rounding, while the paint path rounds each
action's width individually using roundedScaledMetric. At fractional scales,
this divergence causes the next button to extend beyond buttonNode_. Ensure that
when summing the individual widths (prevWidth and nextWidth, obtained from
roundedScaledMetric) in the Yoga path, the combined result is also rounded so it
matches the total width painted by the individual rounded action widths.

In `@src/ui/classic/theme.cpp`:
- Around line 787-845: The overlay clipping and containment calculations around
clipRect and rect currently truncate scaled fractional values through
fcitx::Rect::setPosition/setSize. Keep overlayClipMarginLeft, clipWidth,
overlayWidth, overlayHeight, x, and y in floating-point geometry for
intersection and contains checks, converting to integer Rect coordinates only
for the final rendering operation.
- Around line 334-342: Update the auto-size assignments in the height/width
calculation to round the scaled metric plus margins using the existing
roundedScaledMetric or equivalent std::lround conversion, rather than implicitly
truncating the double result when assigning to int. Preserve the current
fallback conditions and dimension components.

In `@src/ui/classic/xcbinputwindow.cpp`:
- Line 171: Update XCBInputWindow::calculatePositionX and calculatePositionY to
scale each shadowMargin value through roundedScaledMetric using metricScale()
before subtracting it, covering left, right, top, and bottom margins while
preserving the existing positioning logic.
- Around line 211-213: Update the mask/blur-region refresh logic around
parent_->theme().mask() to also track the applied metricScale() (or equivalent
DPI value). Rebuild and reapply the blur property whenever that scale changes,
even when width and height remain unchanged, while preserving the existing
size-change behavior.

In `@src/ui/classic/xcbmenu.cpp`:
- Around line 505-517: In the item.region_ setSize call, replace the second
vertical margin contribution with highlightMargin.marginBottom instead of
repeating highlightMargin.marginTop, while leaving the width and top-margin
calculation unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 7a2773dc-d548-48c7-b46f-1b837bc93a42

📥 Commits

Reviewing files that changed from the base of the PR and between 98058cf and 6f40661.

📒 Files selected for processing (9)
  • src/ui/classic/common.h
  • src/ui/classic/inputwindow.cpp
  • src/ui/classic/inputwindow.h
  • src/ui/classic/theme.cpp
  • src/ui/classic/theme.h
  • src/ui/classic/xcbinputwindow.cpp
  • src/ui/classic/xcbinputwindow.h
  • src/ui/classic/xcbmenu.cpp
  • src/ui/classic/xcbmenu.h

Comment thread src/ui/classic/inputwindow.cpp Outdated
Comment thread src/ui/classic/theme.cpp
Comment thread src/ui/classic/theme.cpp Outdated
Comment thread src/ui/classic/xcbinputwindow.cpp Outdated
Comment thread src/ui/classic/xcbinputwindow.cpp Outdated
Comment thread src/ui/classic/xcbmenu.cpp Outdated
@1024th

1024th commented Jul 31, 2026

Copy link
Copy Markdown
Author

Updated the PR to address the review comments:

  • Made the Yoga prev/next button width match the painted rounded widths:
    YGNodeStyleSetWidth(buttonNode_.get(),
    roundedScaledMetric(prev.width(), scale) +
    roundedScaledMetric(next.width(), scale));
  • Rounded natural-size tile dimensions instead of truncating scaled values:
    if (height < 0) {
    height = static_cast<int>(
    std::lround(scaledMetric(resizeHeight, targetScale) +
    targetMarginTop + targetMarginBottom));
    }
    if (width < 0) {
    width = static_cast<int>(
    std::lround(scaledMetric(resizeWidth, targetScale) +
    targetMarginLeft + targetMarginRight));
  • Scaled shadow margins used for XCB input window positioning:
    const auto scale = metricScale();
    int leftSW = roundedScaledMetric(
    theme.inputPanel->shadowMargin->marginLeft.value(), scale);
    int rightSW = roundedScaledMetric(
    theme.inputPanel->shadowMargin->marginRight.value(), scale);
    int actualWidth = width() - leftSW - rightSW;
    actualWidth = actualWidth <= 0 ? width() : actualWidth;
    if (closestScreen != nullptr) {
    int newX = std::max(x, closestScreen->left());
    if (newX + actualWidth > closestScreen->right()) {
    newX = closestScreen->right() - actualWidth;
    }
    x = std::max(newX, closestScreen->left());
    }
    // exclude shadow border width
    x -= leftSW;
    return x;
    }
    int XCBInputWindow::calculatePositionY(const Rect &cursorRect,
    const Rect *closestScreen) const {
    // TODO: RTL support.
    int y = cursorRect.top();
    auto &theme = parent_->theme();
    const auto scale = metricScale();
    int topSW = roundedScaledMetric(
    theme.inputPanel->shadowMargin->marginTop.value(), scale);
    int bottomSW = roundedScaledMetric(
    theme.inputPanel->shadowMargin->marginBottom.value(), scale);
  • Rebuilt the blur region when the DPI scale changes, not only when the window size changes:
    void XCBInputWindow::updateBlurRegion(int width, int height, double scale) {
    if (!atomBlur_) {
    return;
    }
    blurScale_ = scale;
    Rect rect(0, 0, width, height);
    shrink(rect, *ui_->parent()->theme().inputPanel->blurMargin, scale);
    if (!*ui_->parent()->theme().inputPanel->enableBlur || rect.isEmpty()) {
    xcb_delete_property(ui_->connection(), wid_, atomBlur_);
    return;
    }
    std::vector<uint32_t> data;
    if (ui_->parent()->theme().inputPanel->blurMask->empty()) {
    data.push_back(rect.left());
    data.push_back(rect.top());
    data.push_back(rect.width());
    data.push_back(rect.height());
    } else {
    auto region = parent_->theme().mask(parent_->theme().maskConfig(),
    width, height, scale);
    for (const auto &rect : region) {
    data.push_back(rect.left());
    data.push_back(rect.top());
    data.push_back(rect.width());
    data.push_back(rect.height());
    }
    }
    xcb_change_property(ui_->connection(), XCB_PROP_MODE_REPLACE, wid_,
    atomBlur_, XCB_ATOM_CARDINAL, 32, data.size(),
    data.data());
    }
    void XCBInputWindow::update(InputContext *inputContext) {
    if (!wid_) {
    return;
    }
    auto oldVisible = visible();
    if (inputContext) {
    updateDPI(inputContext);
    }
    auto [width, height] = InputWindow::update(inputContext);
    if (!visible()) {
    if (oldVisible) {
    xcb_unmap_window(ui_->connection(), wid_);
    hoverIndex_ = -1;
    }
    return;
    }
    const auto scale = metricScale();
    bool sizeChanged = width != this->width() || height != this->height();
    bool blurScaleChanged = atomBlur_ && scale != blurScale_;
    if (sizeChanged) {
    resize(width, height);
    }
    if (sizeChanged || blurScaleChanged) {
    updateBlurRegion(width, height, scale);
  • Used the bottom highlight margin for XCB menu item region height (this looks like an existing bug in the original code):
    .setSize(
    maxItemWidth +
    roundedScaledMetric(*highlightMargin.marginLeft, scale) +
    roundedScaledMetric(*highlightMargin.marginRight, scale),
    maxItemHeight +
    roundedScaledMetric(*highlightMargin.marginTop, scale) +
    roundedScaledMetric(*highlightMargin.marginBottom, scale));

@wengxt

wengxt commented Aug 1, 2026

Copy link
Copy Markdown
Member

The change could mostly be minimized to xcb part if you just simply downscale the X11 event coordinates and it shall be similar to wayland model.

@1024th

1024th commented Aug 1, 2026

Copy link
Copy Markdown
Author

Thanks for the suggestion. I refactored the patch following this approach.

Theme layout and Pango metrics now stay in logical coordinates. The XCB backend uses Cairo device scale for rendering, scales only X11-specific window geometry and regions, and converts X11 pointer coordinates back to logical coordinates before passing them to the shared UI code.

@1024th 1024th changed the title Scale classic UI XCB theme metrics with DPI Add HiDPI support to classic UI XCB windows Aug 1, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/ui/classic/xcbinputwindow.cpp`:
- Around line 110-124: The flip-path calculations near newY must scale both
hardcoded 40-unit offsets into physical coordinates. Update the two 40 values
used with newY/actualHeight to use physicalFromLogical(40), preserving the
existing flip behavior and other coordinate calculations.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 3e07cce9-282f-4f0e-9ed7-00672745ddfa

📥 Commits

Reviewing files that changed from the base of the PR and between bc071e2 and cf54a74.

📒 Files selected for processing (6)
  • src/ui/classic/xcbinputwindow.cpp
  • src/ui/classic/xcbinputwindow.h
  • src/ui/classic/xcbmenu.cpp
  • src/ui/classic/xcbmenu.h
  • src/ui/classic/xcbwindow.cpp
  • src/ui/classic/xcbwindow.h

Comment thread src/ui/classic/xcbinputwindow.cpp
@1024th

1024th commented Aug 1, 2026

Copy link
Copy Markdown
Author

Changed the fixed 40-pixel offsets to physicalFromLogical(40).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants